West Midlands | ITP-Sept-25 | Mustaf Asani | Sprint 2 | Data groups coursework #934
Conversation
…n implement folder
…works as expected
…in querystring test suite
asaniDev
added
📅 Sprint 2
cjyuan
added
Reviewed
asaniDev
added
the
Needs Review
cjyuan
removed
the
Needs Review
in querystring file and refactored tally.js to use itemcount
asaniDev
added
Needs Review
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
You are doing great. tally.js could still use some improvement.
Sprint-2/implement/tally.js
Outdated
| let n = 0; | ||
|
|
||
| while (inputArray.length > 0) { | ||
| itemCount = 0; |
There was a problem hiding this comment.
itemCount is used only inside the while loop, so it is best practice to declare the variable here (in the block { ... } where the variable is being used)
Sprint-2/implement/tally.js
Outdated
|
|
||
| while (inputArray.length > 0) { | ||
| itemCount = 0; | ||
| const tempArray = []; |
There was a problem hiding this comment.
Can your code work without using tempArray?
There was a problem hiding this comment.
i realised the danger of nested while loops that rely on internally changing variables, when i changed to remove tempArray as its value was no longer being used, I came across an endless loop as my original one was relying on taking out items from inputArray to give an exit condition. I refactored my code to use an iterator to count the frequency of each item without mutating inputArray.
Sprint-2/implement/tally.js
Outdated
| i--; | ||
| } | ||
|
|
||
| if (itemCount > 0) { |
There was a problem hiding this comment.
Is this check necessary? Under what circumstances will itemCount be 0 or less?
cjyuan
removed
the
Needs Review
…of items in an array.
asaniDev
added
the
Needs Review
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
The previous solution was not as efficient but it was a good opportunity to practice using nested loops.
I will go ahead and approve this PR first, you can improve your solution later.
| throw new Error("error invalid input passed, please provide an array"); | ||
| } | ||
|
|
||
| const tallyObject = {}; |
There was a problem hiding this comment.
Does the following function call returns the value you expect?
tally(["toString", "toString"]);
Suggestion: Look up an approach to create an empty object with no inherited properties.
cjyuan
added
Complete
2 participants
Learners, PR Template
Self checklist
Changelist
Completed the debug, implement and interpret tasks